Implement P2328R1 `join_view` Should Join All Views Of Ranges by miscco · Pull Request #2038 · microsoft/STL

miscco · 2021-07-03T12:02:06Z

Fixes #1985

I am still baffled why array<string, 5>{{{}, "Hello ", {}, "World!", {}}} | views::join was not fixed 😿

Fixes microsoft#1985

miscco · 2021-07-03T12:18:04Z

Derp, I did not update the can_test boolean that enables the test...

stl/inc/ranges

timsong-cpp

I am still baffled why array<string, 5>{{{}, "Hello ", {}, "World!", {}}} | views::join was not fixed 😿

I see nothing to fix, other than perhaps making array a view?

I also note that the paper title is incorrectly transcribed.

stl/inc/ranges

Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Co-authored-by: Tim Song <t.canens.cpp@gmail.com>

stl/inc/ranges

tests/std/tests/P0896R4_views_join/test.cpp

CaseyCarter · 2021-07-15T20:48:22Z

I also note that the paper title is incorrectly transcribed.

I'm tempted to veto the Stephan-casing of the title; "Views of Ranges" doesn't have the same meaning as the intended "views of ranges". @StephanTLavavej, could you live with "P2328R11 join_view Should Join All views Of ranges"?

stl/inc/ranges

Co-authored-by: Casey Carter <cacarter@microsoft.com>

tests/std/tests/P0896R4_views_join/test.cpp

stl/inc/ranges

tests/std/tests/P0896R4_views_join/test.cpp

Remove extraneous blank line.

CaseyCarter · 2021-08-05T02:32:03Z

@miscco FYI I pushed a change that removed all of the _Dummy members from the optional-alike types in <ranges>. We don't need them in C++20, since we can get away with initializing no union member even in a constexpr constructor.

stl/inc/ranges

tests/std/tests/P0896R4_views_join/test.cpp

StephanTLavavej

Looks good to me, after @timsong-cpp's comments are addressed.

miscco · 2021-08-05T14:08:56Z

@timsong-cpp I think I managed to get it working as you suggested.

stl/inc/ranges

StephanTLavavej

Looks good, I'll push a trivial change to take (int) instead of (const int).

tests/std/tests/P0896R4_views_join/test.cpp

StephanTLavavej · 2021-08-07T00:22:04Z

FYI @CaseyCarter, @miscco and I pushed changes after you approved.

StephanTLavavej · 2021-08-09T22:08:55Z

Mirrored to internal MSVC-PR-343332. Please notify me if any further changes are pushed to this branch.

StephanTLavavej · 2021-08-09T23:54:03Z

@miscco @CaseyCarter This failed in the internal test harness so I had to drop the || defined(MSVC_INTERNAL_TESTING) logic.

StephanTLavavej · 2021-08-10T23:51:13Z

Thanks for joining this project! 😹 🎉 🚀

miscco requested a review from a team as a code owner July 3, 2021 12:02

Implement P2328R1 join_view should join all ranges

92ce96e

Fixes microsoft#1985

miscco force-pushed the P2328-join-view branch from 433caa3 to 92ce96e Compare July 3, 2021 12:03

AdamBucior reviewed Jul 3, 2021

View reviewed changes

stl/inc/ranges Show resolved Hide resolved

Specialize _Non_propagating_cache and fix test

49bf9a3

miscco force-pushed the P2328-join-view branch from 150a851 to 49bf9a3 Compare July 3, 2021 18:56

miscco commented Jul 3, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

miscco commented Jul 3, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

miscco commented Jul 3, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

AdamBucior reviewed Jul 3, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

miscco added 2 commits July 3, 2021 21:53

More tests

5e74f42

We should rela test requirements

8041fc4

timsong-cpp suggested changes Jul 3, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

StephanTLavavej added cxx20 C++20 feature defect report Applied retroactively ranges C++20/23 ranges labels Jul 3, 2021

miscco and others added 2 commits July 4, 2021 12:55

More test coverage and internal testing

351f538

Address more review comments

72f392c

Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Co-authored-by: Tim Song <t.canens.cpp@gmail.com>

eldakesh-ms reviewed Jul 14, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

CaseyCarter self-assigned this Jul 14, 2021

miscco added 2 commits July 15, 2021 21:45

Properly destroy existing element in _Emplace_deref

e283371

Further simplify _Non_propagating_cache for trivially_destructible types

f546493

CaseyCarter suggested changes Jul 15, 2021

View reviewed changes

CaseyCarter changed the title ~~Implement P2328R1 join_view should join all ranges~~ Implement P2328R1 join_view Should Join All Views Of Ranges Jul 15, 2021

CaseyCarter reviewed Jul 15, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

CaseyCarter assigned miscco and unassigned CaseyCarter Jul 15, 2021

More review comments

c2bc106

Co-authored-by: Casey Carter <cacarter@microsoft.com>

Address review comments

317fe66

miscco force-pushed the P2328-join-view branch from ec602b5 to 317fe66 Compare August 4, 2021 13:20

CaseyCarter reviewed Aug 4, 2021

View reviewed changes

tests/std/tests/P0896R4_views_join/test.cpp Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

tests/std/tests/P0896R4_views_join/test.cpp Outdated Show resolved Hide resolved

Update tests/std/tests/P0896R4_views_join/test.cpp

2075c52

Remove extraneous blank line.

StephanTLavavej self-assigned this Aug 4, 2021

Remove "dummy" union members not needed for C++20

3787596

CaseyCarter approved these changes Aug 5, 2021

View reviewed changes

CaseyCarter removed their assignment Aug 5, 2021

timsong-cpp reviewed Aug 5, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

stl/inc/ranges Outdated Show resolved Hide resolved

tests/std/tests/P0896R4_views_join/test.cpp Show resolved Hide resolved

StephanTLavavej requested changes Aug 5, 2021

View reviewed changes

Enable join_view to handle immovable types

f9a7a6a

timsong-cpp reviewed Aug 5, 2021

View reviewed changes

stl/inc/ranges Outdated Show resolved Hide resolved

Merge branch 'main' into P2328-join-view

52d36e1

miscco force-pushed the P2328-join-view branch from 030d633 to 52d36e1 Compare August 6, 2021 09:37

Hijack the _Not_quite_object::_Construct_tag to avoid conversions

0c54f03

StephanTLavavej reviewed Aug 6, 2021

View reviewed changes

tests/std/tests/P0896R4_views_join/test.cpp Outdated Show resolved Hide resolved

Remove unnecessary const on a nameless value parameter.

b3925ac

StephanTLavavej removed their assignment Aug 6, 2021

StephanTLavavej approved these changes Aug 6, 2021

View reviewed changes

StephanTLavavej self-assigned this Aug 9, 2021

Remove || defined(MSVC_INTERNAL_TESTING).

4a7ba61

StephanTLavavej merged commit 7f4cc9e into microsoft:main Aug 10, 2021

StephanTLavavej removed their assignment Aug 11, 2021

miscco deleted the P2328-join-view branch August 11, 2021 09:41

Comments

Conversation

miscco commented Jul 3, 2021

Uh oh!

miscco commented Jul 3, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timsong-cpp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CaseyCarter commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CaseyCarter commented Aug 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StephanTLavavej left a comment

Choose a reason for hiding this comment

Uh oh!

miscco commented Aug 5, 2021

Uh oh!

Uh oh!

StephanTLavavej left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StephanTLavavej commented Aug 7, 2021

Uh oh!

StephanTLavavej commented Aug 9, 2021

Uh oh!

StephanTLavavej commented Aug 9, 2021

Uh oh!

StephanTLavavej commented Aug 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CaseyCarter commented Jul 15, 2021 •

edited

Loading